Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/streamlink and live #659

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

CMorrison82z
Copy link

Implements #658

See annotated code changes. I expect what I've done can be improved.

@CMorrison82z
Copy link
Author

CMorrison82z commented Dec 13, 2024

Beware that I've left a few things a bit of a mess. Wasn't sure how you'd want things organized / implemented so I left them as is.

@CMorrison82z
Copy link
Author

Config for what player to use and stream quality would be good

@Xithrius Xithrius self-requested a review December 13, 2024 23:16
@Xithrius
Copy link
Owner

Xithrius commented Dec 13, 2024

Beware that I've left a few things a bit of a mess. Wasn't sure how you'd want things organized / implemented so I left them as is.

Understandable.

I've had a quick look over the PR, and it conflicts about my intuition for how this could be implemented. The following widget takes from the twitch API, so based on a config/state value the API call could probably just be switched out and the UI could be changed a little to show a tab/header based on if you're currently searching by who you're following and who's live. I see that you already have the logic for that, but I don't think this requires changes of higher level component structures. TLDR change following widget to do a different API call based on config/state value.

Config for what player to use and stream quality would be good

I like that idea. If you want to make a tracking issue for that, go ahead.

@CMorrison82z
Copy link
Author

I can't remember exactly, but TwitchAction being in TerminalAction was causing me issues in SearchWidget when I was messing with how to search & return the selected search item. Also I needed to create a matching struct for the API response to a different call. I'm sure there's a more elegant / straightforward way of implementing this change, this is just what I saw after skimming the source code myself. Feel free to implement this how you see fit.

Also, you probably want to checkout what I'm doing with spawning streamlink and holding onto that process handle, because I didn't really know what I was doing.

@Xithrius
Copy link
Owner

I'll take a longer stare in the near future, thank you for the initial changes!

@Xithrius
Copy link
Owner

Xithrius commented Jan 1, 2025

Hey, sorry I haven't gotten around to this (IRL stuff). Hope to get a proper review going soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants